Conversation
merge from hpc/clusters
Project.toml
Outdated
| Adapt = "79e6a3ab-5dfb-504d-930d-738a2a938a0e" | ||
| CUDA = "052768ef-5323-5732-b1bb-66c8b64840ba" | ||
| Combinatorics = "861a8166-3701-5b0c-9a16-15d98fcdc6aa" | ||
| DataGraphs = "b5a273c3-7e6c-41f6-98bd-8d7f1525a36a" |
There was a problem hiding this comment.
Is this dependency used? I removed anything requiring it a few version ago
There was a problem hiding this comment.
No, it's not used, I'll remove it.
Project.toml
Outdated
|
|
||
| [deps] | ||
| Adapt = "79e6a3ab-5dfb-504d-930d-738a2a938a0e" | ||
| CUDA = "052768ef-5323-5732-b1bb-66c8b64840ba" |
There was a problem hiding this comment.
This shouldn't be a hard dependency (requires a GPU)
Project.toml
Outdated
| ITensors = "9136182c-28ba-11e9-034c-db9fb085ebd5" | ||
| KrylovKit = "0b1a1467-8014-51b9-945f-bf0ae24f4b77" | ||
| LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" | ||
| MKL = "33e6dc65-8f57-5167-99aa-e5a354878fb2" |
There was a problem hiding this comment.
| MKL = "33e6dc65-8f57-5167-99aa-e5a354878fb2" |
Project.toml
Outdated
| KrylovKit = "0b1a1467-8014-51b9-945f-bf0ae24f4b77" | ||
| LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" | ||
| MKL = "33e6dc65-8f57-5167-99aa-e5a354878fb2" | ||
| NPZ = "15e1cf62-19b3-5cfa-8e77-841668bca605" |
There was a problem hiding this comment.
| NPZ = "15e1cf62-19b3-5cfa-8e77-841668bca605" |
Project.toml
Outdated
| NPZ = "15e1cf62-19b3-5cfa-8e77-841668bca605" | ||
| NamedGraphs = "678767b0-92e7-4007-89e4-4527a8725b19" | ||
| PauliPropagation = "293282d5-3c99-4fb6-92d0-fd3280a19750" | ||
| ProgressMeter = "92933f4c-e287-5a05-a399-4b506db050ca" |
There was a problem hiding this comment.
| ProgressMeter = "92933f4c-e287-5a05-a399-4b506db050ca" |
| function update(alg::Algorithm"bp", bpc::AbstractBeliefPropagationCache) | ||
| compute_error = !isnothing(alg.kwargs.tolerance) | ||
| if !compute_error | ||
| println("ALERT, not computing error") |
There was a problem hiding this comment.
This forces tolerance to always be defined. But it maybe be the case the user just wants to run BP for a fixed number of iterations and doesn't want to know convergence info
There was a problem hiding this comment.
It doesn't force it to be defined, just prints a message. But sure I can remove it.
src/MessagePassing/loopcorrection.jl
Outdated
| end | ||
|
|
||
| #Get the all edges incident to the region specified by the vector of edges passed | ||
| # Optionally pass in a vector of vertices in the region, to handle the corner case where the region is just one vertex. |
There was a problem hiding this comment.
If you region is vertex based instead, this can be handled by boundary_edges(bpc::BeliefPropagationCache, verts::Vector) which should be an existing function (as there is a NamedGraphs version of it)
| end | ||
| break | ||
| if compute_error | ||
| diffs[i] = diff.x |
There was a problem hiding this comment.
It gets printed out in line 222. In a previous version I was returning it along with bpc, but that seemed like too much of a breaking change.
There was a problem hiding this comment.
There is a print statement at the end of the function where the entire vector is printed. In an earlier version I also returned diffs, but that affects too many other functions so I opted not to.
| end | ||
|
|
||
| struct Loop | ||
| vertices::Vector{Int} |
There was a problem hiding this comment.
This assumes the vertices are Int in the loop? But can't we label them arbitrarily like we do with a NamedGraph
There was a problem hiding this comment.
In the cluster finding, we convert to a SimpleGraph first, which has integer vertices.
There was a problem hiding this comment.
But I just changed to Vector anyway.
| is_proper_subset(a::RegionKey, b::RegionKey)::Bool | ||
| Return true if region `a` is a proper subset of `b`. | ||
| """ | ||
| function is_proper_subset(a::RegionKey, b::RegionKey)::Bool |
There was a problem hiding this comment.
Can the Julia function issubset not handle this?
There was a problem hiding this comment.
Good point. I just copied this from the code you shared with me before. Technically, issubset(a,b) also returns true if a==b, but I can just add that as a separate check.
There was a problem hiding this comment.
Yeah could have
is_proper_subset(a::RegionKey, b::RegionKey) = issubset(a,b) && !issetequal(a,b)
|
Thanks Grace! How much work is the |
|
|
|
Yeah maybe it's best to drop the Hyperdualnumbers dependency then and go with the ratio approach? It's probably more efficient like you said and clearer from a code perspective (also removes a dependency) |
|
|
||
| This is overkill as it finds ALL subgraphs first, but my other implementation had bugs | ||
| """ | ||
| function enumerate_clusters(ng::NamedGraph, max_weight::Int; min_v::Int = 4, triangle_free::Bool = true, must_contain = [], min_deg::Int = 2, verbose::Bool = false) |
There was a problem hiding this comment.
So this should definitely return the clusters with vertex labelling that ng has.
This is how I do it for edge_color (basically define the function on a SimpleGraph and then overload it on the NamedGraph by unwrapping the graph and re-wrapping the result:
function SimpleGraphAlgorithms.edge_color(g::AbstractNamedGraph, k::Int64)
pg, vs = position_graph(g), collect(vertices(g))
ec_dict = SimpleGraphAlgorithms.edge_color(UndirectedGraph(pg), k)
# returns k vectors of edges which each contain the colored/commuting edges
return [
[edgetype(g)(vs[first(first(e))], vs[last(first(e))]) for e in ec_dict if last(e) == i]
for i in 1:k
]
end
end
There was a problem hiding this comment.
I think I already handle the vertex labeling in the function generalized_loop_named? Is there an issue with the existing approach?
Added code for cluster expansion and cluster cumulant expansion. Calculation of one- and two-point functions. Example code in
examples/clustercorrections.jl.Note: generalized loop finding in cluster expansion relies on
nautywhich must be downloaded here.Other changes:
freenergytofree_energy.message_diffno longer returns negative numbers.Project.toml.